Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: login in with a second account during a call via deep links (WPB-8937) #2948

Merged
merged 7 commits into from
May 2, 2024
Merged

fix: login in with a second account during a call via deep links (WPB-8937) #2948

merged 7 commits into from
May 2, 2024

Conversation

ohassine
Copy link
Member

@ohassine ohassine commented Apr 29, 2024 •
edited by github-actions bot
Loading

BugWPB-8937 [Android] Deeplink can bypass adding a new account during a call

PR Submission Checklist for internal contributors

  • The PR Title

    • conforms to the style of semantic commits messages¹ supported in Wire's Github Workflow²
    • contains a reference JIRA issue number like SQPIT-764
    • answers the question: If merged, this PR will: ... ³

  • The PR Description

    • is free of optional paragraphs and you have filled the relevant parts to the best of your ability

What's new in this PR?

Issues

While in a call, users can login to another account via deep links

Solutions

Show a toast instead of login dialog in case there an ongoing call

Needs releases with:

  • GitHub link to other pull request

Testing

Test Coverage (Optional)

  • I have added automated test to this contribution

How to Test

Briefly describe how this change was tested and if applicable the exact steps taken to verify that it works as expected.

Notes (Optional)

Specify here any other facts that you think are important for this issue.

Attachments (Optional)

Attachments like images, videos, etc. (drag and drop in the text box)

PR Post Submission Checklist for internal contributors (Optional)

  • Wire's Github Workflow has automatically linked the PR to a JIRA issue

PR Post Merge Checklist for internal contributors

  • If any soft of configuration variable was introduced by this PR, it has been added to the relevant documents and the CI jobs have been updated.
References
  1. https://sparkbox.com/foundry/semantic_commit_messages
  2. https://github.com/wireapp/.github#usage
  3. E.g. feat(conversation-list): Sort conversations by most emojis in the title #SQPIT-764.

@ohassine ohassine requested review from a team, yamilmedina, borichellow, MohamadJaara, Garzas and saleniuk and removed request for a team April 29, 2024 10:47
alexandreferris
alexandreferris approved these changes Apr 29, 2024
View reviewed changes
Copy link
Contributor

@alexandreferris alexandreferris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch! Just a nitpick on naming

app/src/main/kotlin/com/wire/android/ui/WireActivityViewModel.kt Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Apr 29, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 44.25%. Comparing base (70f61a5) to head (5ae4106).
Report is 2 commits behind head on develop.

Additional details and impacted files 
@@             Coverage Diff             @@
##           develop    #2948      +/-   ##
===========================================
+ Coverage    44.04%   44.25%   +0.21%     
===========================================
  Files          444      444              
  Lines        14449    14458       +9     
  Branches      2473     2476       +3     
===========================================
+ Hits          6364     6399      +35     
+ Misses        7398     7364      -34     
- Partials       687      695       +8
Files Coverage Δ
...rc/main/kotlin/com/wire/android/ui/WireActivity.kt 0.00% <ø> (ø)
...otlin/com/wire/android/ui/WireActivityViewModel.kt 70.35% <100.00%> (+0.96%) ⬆️
...om/wire/android/util/deeplink/DeepLinkProcessor.kt 75.28% <100.00%> (ø)

... and 5 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 831f1b5...5ae4106. Read the comment docs.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Apr 29, 2024 •
edited
Loading

Test Results

924 tests   924 ✅  13m 37s ⏱️
123 suites    0 💤
123 files      0 ❌

Results for commit 5ae4106.

♻️ This comment has been updated with latest results.

@github-actions GitHub Actions
Copy link
Contributor

APKs built during tests are available here. Scroll down to Artifacts!

@github-actions GitHub Actions
Copy link
Contributor

APKs built during tests are available here. Scroll down to Artifacts!

@AndroidBob
Copy link
Collaborator

Build 4526 succeeded.

The build produced the following APK's:

@github-actions GitHub Actions
Copy link
Contributor

APKs built during tests are available here. Scroll down to Artifacts!

@AndroidBob
Copy link
Collaborator

Build 4532 succeeded.

The build produced the following APK's:

@MohamadJaara MohamadJaara changed the base branch from develop to release/candidate May 1, 2024 00:12
@pull-request-size pull-request-size bot added size/M and removed size/S labels May 1, 2024
@MohamadJaara MohamadJaara changed the base branch from release/candidate to develop May 1, 2024 00:13
@pull-request-size pull-request-size bot added size/S and removed size/M labels May 1, 2024
@ohassine ohassine changed the title fix: login in a second account during a call via deep links (WPB-8937) fix: login in with a second account during a call via deep links (WPB-8937) May 2, 2024
@ohassine ohassine added this pull request to the merge queue May 2, 2024
Merged via the queue into develop with commit d099c02 May 2, 2024
18 checks passed
@ohassine ohassine deleted the fix-login-during-call-via-deeplinks branch May 2, 2024 09:14
ohassine added a commit that referenced this pull request May 2, 2024
@ohassine
fix: login in with a second account during a call via deep links (WPB…
7e3891a 
…-8937) (#2948)

(cherry picked from commit d099c02)
@ohassine ohassine mentioned this pull request May 2, 2024
Merged
8 tasks
@github-actions github-actions bot mentioned this pull request May 2, 2024
Closed
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexandreferris alexandreferris alexandreferris approved these changes

@yamilmedina yamilmedina yamilmedina approved these changes

@borichellow borichellow Awaiting requested review from borichellow borichellow was automatically assigned from wireapp/android

@MohamadJaara MohamadJaara Awaiting requested review from MohamadJaara MohamadJaara was automatically assigned from wireapp/android

@Garzas Garzas Awaiting requested review from Garzas Garzas was automatically assigned from wireapp/android

@saleniuk saleniuk Awaiting requested review from saleniuk saleniuk was automatically assigned from wireapp/android

Assignees

@ohassine ohassine

Labels
size/S
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ohassine @codecov-commenter @AndroidBob @yamilmedina @alexandreferris